Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin State Converter rework #454

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

abique
Copy link
Contributor

@abique abique commented Feb 5, 2025

No description provided.

@abique abique self-assigned this Feb 5, 2025
clap_id src_param_id,
clap_id *dst_param_ids,
uint32_t dst_param_ids_size);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get_mapping is reduntant I think? convert_value can do this already.

uint32_t flags,
clap_plugin_state_converter_param_value_t *src,
clap_plugin_state_converter_param_value_t *dsts,
uint32_t dsts_size);
} clap_plugin_state_converter_t;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to turn convert_state, get_mapping and convert_value into a single function:

int32_t(CLAP_ABI *convert)(struct clap_plugin_state_converter *converter,
uint32_t flags,
const clap_istream_t *src,
const clap_ostream_t *dst,
clap_plugin_state_converter_param_value_t *srcs,
uint32_t srcs_count,
clap_plugin_state_converter_param_value_t *dsts,
uint32_t dsts_size);

This avoids the rather ugly "convert_value assumes the state of the plugin to the one from the most recent call to". It also gives plugin developers the whole shebang in one go, so we don't have to change it again when a developer needs to see more than one automation point to be able to do the right thing.

@abique abique force-pushed the next branch 2 times, most recently from eff5302 to 00113aa Compare February 18, 2025 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants